fix: add transaction cache to L1 watcher#433
Conversation
CodSpeed Performance ReportMerging #433 will not alter performanceComparing Summary
|
jonastheis
left a comment
jonastheis
left a comment
There was a problem hiding this comment.
After switching to processing logs one at a time, this logic has broken
What exactly do you mean? iiuc we still group by tx hash here:
rollup-node/crates/watcher/src/lib.rs
Line 555 in b6d45d0
| &mut self, | ||
| tx_hash: TxHash, | ||
| ) -> L1WatcherResult<Option<B256>> { | ||
| if let Some(entry) = self.cache.get_mut(&tx_hash) { |
There was a problem hiding this comment.
just wonder why get_transaction_next_blob_versioned_hash also fetches it from the provider when the cache doesn't find it.
There was a problem hiding this comment.
Because get_transaction_next_blob_versioned_hash is a stateful process, I thought it would reduce errors if we made the process more explicit and required a specific sequence.
There was a problem hiding this comment.
Could it be that there is no need to call get_transaction_next_blob_versioned_hash separately in the future?
There was a problem hiding this comment.
Yes, it is possible we can remove this method in the future with a bit of refactoring.
3 participants
Overview
This PR addresses an issue related to the watcher. We have moved to a pattern in which we process logs in the order in which they have been produced on-chain, this is to maintain the same order when producing
L1Notifications. There is an edge case in thehandle_batch_commitsfunction in which we need to know the relative position of the blob versioned hash associated with the batch commit log. This is achieved by being aware of the index of theBatchCommitlog associated with the transaction. After switching to processing logs one at a time, this logic has broken. We fix this by introducing a cache, which stores the transaction and its associated blob versioned hashes. The cache exposes a methodget_transaction_next_blob_versioned_hashwhich maintains the index of the blob versioned hash as we iterate through the logs, such that we can make this association.Note
I have successfully synced the chain using this PR.